Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Method stating which commit is being played during an halted rebase #903

Merged
merged 4 commits into from Aug 14, 2019
Merged

Method stating which commit is being played during an halted rebase #903

merged 4 commits into from Aug 14, 2019

Conversation

Arthur-Milchior
Copy link
Contributor

This will be useful to me at least. This way, I know that I can tell
my script to omit some specific commits. If you accept to merge it, I
may also do similar method for merges and cherry pick.

@Arthur-Milchior
Method stating which commit is being played during an halted rebase
22a1f29 
This will be useful to me at least. This way, I know that I can tell
my script to omit some specific commits. If you accept to merge it, I
may also do similar method for merges and cherry pick.
@ghost
Copy link

ghost commented Aug 11, 2019

Thanks a lot, good idea! Great you took the extra time to make a contribution.

I would happily merge, but believe it needs some work before that could happen.

  • camel case method name should be snake case
  • Heads or commits have native types, do you think it would be more convenient to return such object instead of string?
  • please add tests for both code paths

I understand that this would mean quite some extra work, and hope you can make it work.

Besides, I am happily accepting additional quality of life methods if they meet the quality requirements.

Thanks for your consideration:)

@codecov-io
Copy link

codecov-io commented Aug 11, 2019 •
edited

Codecov Report

Merging #903 into master will decrease coverage by 1.3%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
- Coverage   94.84%   93.54%   -1.31%     
==========================================
  Files          59       59              
  Lines        9766     9792      +26     
==========================================
- Hits         9263     9160     -103     
- Misses        503      632     +129
Impacted Files Coverage Δ
git/repo/base.py 93.04% <87.5%> (-2.37%) ⬇️
git/compat.py 41.21% <0%> (-23.61%) ⬇️
git/test/test_util.py 90.66% <0%> (-8%) ⬇️
git/test/lib/asserts.py 61.53% <0%> (-7.7%) ⬇️
git/test/lib/helper.py 85.87% <0%> (-6.26%) ⬇️
git/test/test_base.py 96.59% <0%> (-2.28%) ⬇️
git/test/test_git.py 96.66% <0%> (-2.23%) ⬇️
git/objects/submodule/base.py 93.08% <0%> (-1.42%) ⬇️
git/test/test_repo.py 96.59% <0%> (-1.4%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daa3f35...9334614. Read the comment docs.

@Arthur-Milchior
Copy link
Contributor Author

What do you mean by «both code paths» ? I added a single function, and already added tests for them
I don't know what is a «quality of life method», sorry :s

@Arthur-Milchior
Copy link
Contributor Author

The other two requested change are already done

@Byron
Copy link
Member

Byron commented Aug 12, 2019

Thanks for the changes. The tests would have to be added in test_repo.py, however, I can't see any change to a file related to tests. Both code paths refers to both the success case, and the one where there is no active rebase.

Screenshot 2019-08-12 at 11 01 11

My remark about 'quality of life methods' refers to your comment about adding more methods like this, which I assume are to help you work with GitPython more easily, thus improving your 'quality of life', metaphorically speaking :D.

@Arthur-Milchior
Copy link
Contributor Author

I added the test in a new file, and forgot to add the file to git. My bad 🈂️
Now the test is in test_repo, as you asked for.

@Byron Byron added this to the v3.0.1 - Bugfixes milestone Aug 14, 2019
@Byron
Copy link
Member

Byron commented Aug 14, 2019

Thanks a lot for your contribution! That does the trick!

@Byron Byron merged commit f6fdb67 into gitpython-developers:master Aug 14, 2019
Byron pushed a commit that referenced this pull request Aug 14, 2019
@Arthur-Milchior @Byron
Snack case as requested in #903
87b5d9c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
v3.0.1 - Bugfixes
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Arthur-Milchior @codecov-io @Byron